Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set TCP keepalive on inbound clusterbus connections #9230

Merged
merged 16 commits into from Jul 16, 2021
Merged

Set TCP keepalive on inbound clusterbus connections #9230

merged 16 commits into from Jul 16, 2021

Conversation

qetu3790
Copy link
Contributor

@qetu3790 qetu3790 commented Jul 13, 2021

Addresses the issue where an inbound link is disconnected but is never detected since the packet was dropped. This can cause a memory leak as the connection will not be detected through any other mechanism.

For example, A and B are two nodes in a cluster.

When A is partitioned away, B will free the "struct clusterLink" which is used for send ping to A. But the corresponding "struct clusterLink" in A won't be freed.

When partition heals, B uses a new clusterLink to send ping to A and A will create another new corresponding clusterLink. The older "struct clusterLink" above is still in the memory.

For example, A and B are two nodes in a cluster.

When A is partitioned away, B will free the "struct clusterLink" which is used for send ping to A. But the corresponding "struct clusterLink" in A won't be freed.

When partition heals, B uses a new clusterLink to send ping to A and A will create another new corresponding clusterLink. The older "struct clusterLink" above is still in the memory.
src/cluster.c Outdated Show resolved Hide resolved
src/cluster.h Outdated Show resolved Hide resolved
src/cluster.c Outdated Show resolved Hide resolved
src/cluster.c Outdated Show resolved Hide resolved
@madolson madolson added state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten release-notes indication that this issue needs to be mentioned in the release notes labels Jul 15, 2021
Co-authored-by: Madelyn Olson <34459052+madolson@users.noreply.github.com>
@madolson madolson changed the title [Fix]Memory leak in cluster Set TCP keepalive on outbound clusterbus commections Jul 16, 2021
@madolson madolson changed the title Set TCP keepalive on outbound clusterbus commections Set TCP keepalive on outbound clusterbus connections Jul 16, 2021
@madolson madolson changed the title Set TCP keepalive on outbound clusterbus connections Set TCP keepalive on inbound clusterbus connections Jul 16, 2021
Copy link
Contributor

@madolson madolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@madolson madolson merged commit f03af47 into redis:unstable Jul 16, 2021
@oranagra
Copy link
Member

@madolson shall we backport this to 6.0 and 6.2?

@madolson
Copy link
Contributor

Backporting seems safe, it's very low risk and should help some edge cases.

@madolson madolson added this to To Do in 6.0 Backport via automation Jul 19, 2021
oranagra pushed a commit to oranagra/redis that referenced this pull request Jul 20, 2021
Set TCP keepalive on inbound clusterbus connections to prevent memory leak

(cherry picked from commit f03af47)
@oranagra oranagra mentioned this pull request Jul 21, 2021
oranagra pushed a commit to oranagra/redis that referenced this pull request Jul 21, 2021
Set TCP keepalive on inbound clusterbus connections to prevent memory leak

(cherry picked from commit f03af47)
@oranagra oranagra mentioned this pull request Jul 21, 2021
oranagra pushed a commit that referenced this pull request Jul 21, 2021
Set TCP keepalive on inbound clusterbus connections to prevent memory leak

(cherry picked from commit f03af47)
oranagra pushed a commit that referenced this pull request Jul 21, 2021
Set TCP keepalive on inbound clusterbus connections to prevent memory leak

(cherry picked from commit f03af47)
@oranagra oranagra moved this from To Do to In progress in 6.0 Backport Jul 22, 2021
@oranagra oranagra moved this from In progress to Done in 6.0 Backport Jul 22, 2021
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Sep 8, 2021
Set TCP keepalive on inbound clusterbus connections to prevent memory leak
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes indication that this issue needs to be mentioned in the release notes state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants